Skip to content

Allows DatasetRestoring to act on fields by different name#199

Open
jagoosw wants to merge 4 commits intomainfrom
jsw/restore-different-name
Open

Allows DatasetRestoring to act on fields by different name#199
jagoosw wants to merge 4 commits intomainfrom
jsw/restore-different-name

Conversation

@jagoosw
Copy link
Copy Markdown
Collaborator

@jagoosw jagoosw commented Apr 30, 2026 •
edited
Loading

This PR moves variable_name to be a kwarg in DatasetRestoring to allow users to override the default naming. A user would be required to define a struct and getindex method for the name, for example:

struct OceanBioMEAlkalinity end
Base.getindex(fields, i, j, k, ::OceanBioMEAlkalinity) = @inbounds fields.Alk[i, j, k]

@jagoosw jagoosw marked this pull request as ready for review April 30, 2026 13:12
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/DataWrangling/ECCO/ECCO.jl 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@simone-silvestri
Copy link
Copy Markdown
Member

simone-silvestri commented May 1, 2026

I don't think we can pass a varname that is different than the one in metadata, while this keyword argument would allow it. I would probably drop the variable_name kwarg, what you want to achieve can be done just by passing field_name am I right?

simone-silvestri
simone-silvestri reviewed May 1, 2026
View reviewed changes
Comment thread src/DataWrangling/restoring.jl Outdated
@jagoosw
Copy link
Copy Markdown
Collaborator Author

jagoosw commented May 1, 2026

I don't think we can pass a varname that is different than the one in metadata, while this keyword argument would allow it. I would probably drop the variable_name kwarg, what you want to achieve can be done just by passing field_name am I right?

Oh yes thats true

@jagoosw @simone-silvestri
Update src/DataWrangling/restoring.jl
6797f94 
Co-authored-by: Simone Silvestri <silvestri.simone0@gmail.com>
@jagoosw
Copy link
Copy Markdown
Collaborator Author

jagoosw commented May 2, 2026

Could I add a scale factor to the dataset? Do you think it would be best to do it in the constructor, or to pass a scale factor to the forcing function?

@jagoosw
Adds fallback for ECCO locaitons
d3dcbc3 
Since 0.3.0 can't build FTS of ECCO Darwin variables as their location isn't in the ECCO_location dictionary
@simone-silvestri
Copy link
Copy Markdown
Member

simone-silvestri commented May 2, 2026

there is already the rate and mask kwargs. Would the scale be different?

@jagoosw
Copy link
Copy Markdown
Collaborator Author

jagoosw commented May 2, 2026

there is already the rate and mask kwargs. Would the scale be different?

I mean if the dataset is different (e.g. in different units) to the field its restoring, so it would go in as s in $r (s\psi_r - \psi)$

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simone-silvestri simone-silvestri simone-silvestri approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jagoosw @simone-silvestri